Allow use of CodeFormatter from no_std
#15
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As a result, this removes the need for a
feature = "std"
, and makes all the code in this crate unconditionallyno_std
(and without even needingextern crate alloc
either).It does this by making CodeFormatter generic over the type of the indentation,
S: AsRef<str>
. It's a breaking change, since now CodeFormatter has another generic parameter.The generic parameter could be more general if instead it allowed
S: fmt::Display
rather thanS: AsRef<str>
. While that actually appeals to me a bit, I think it would not actually be very useful in practice, and it has some performance downsides1 that which run contrary to (if nothing else) the documented goals ofCodeFormatter
(which includes being efficient).A downside of this is that now the type of the indentation string is in the type signature. This arguably is an encapsulation violation (but who cares), and more problematically, it means that if code want to pass a
CodeFormatter<'_, T, &'static str>
to a function expecting aCodeFormatter<'_, T, String>
in the signature, it can't. I don't think this downside actually matters3, but felt like mentioning it because maybe you do.This change invalidated part of the
CodeFormatter
docs, but I didn't know what to rewrite that part to, so I just deleted it. LMK if you have something you thing should go there instead.(Sorry, somehow I wrote several footnote5s for this simple-seeming patch)
1: It's highly likely (if not guaranteed) that
fmt::Write::write_str
is going to be faster than invokingwrite!
, but it also prevents this crate from doing various optimizations which are plausibly desirable. For example, this crate might want want to optimize repeated indents of all-" "
strings2.2: Note: that patch isn't a good way of doing this in retrospect — I realized a much simpler implementation is possible after writing it — but it's an example of the kind of thing that might be desirable, but requires
S: AsRef<str>
, and notS: Display
(at least without access to specialization, unless we make various assumptions that likely defeat the point of usingS: Display
).3: So, mostly I feel this way because its the same as the current situation for
T
. Also, I expect any usage that cares about the flexibility will just make their function generic overS
. This is actually the path of least resistance, given we don't provide a default forS
.That said, I tried to think of ways to alleviate it, since it's certainly plausible someone could hit the issue. I don't think there's a great solution, but the big case I'd be concerned about is allowing code to pass a "child" CodeFormatter to a function that wants to write to it.
There is sort of a solution I came up with to this problem though. Probably anyway... the code below at least compiles4, and lets you get a
CodeFormatter<'_, T, &str>
out of a&mut CodeFormatter<'_, T, S>
(the source must be&mut
to hand the&mut T
off to the child.So, as you can see, I didn't include this. Mostly since IDK why the code that wants this wouldn't just be generic over
S
. Also, I'm not sure I can actually really explain what this does succinctly (it type-erasesS
into a&str
mainly, but... also there's some wonkiness around the fact that it produces what seems like a copy, although writes still go to the same place, but level changes might not work as intended... etc).That said, if you want me to include it, I'm totally happy to.
4: I haven't checked if it can actually be used — I'm not sure if the
'b
lifetime being the same in both places would forbid that... I think it's fine though? I think you'd use it like&mut s.into()
. Maybe this wouldn't infer right though, in which case an explicit name would be needed (no idea what a good name for it would be...)5: They're... substantially longer than the non-footnote text now, and have their own sub-footnotes... I may have a problem... Sorry...